Skip to content

Statically defining LCD classes, removing post-init checks #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Times-Z merged 11 commits into from
Feb 2, 2026
Merged

Statically defining LCD classes, removing post-init checks #21

Times-Z merged 11 commits into from
Feb 2, 2026

Conversation

@Electro707
Copy link
Contributor

@Electro707 Electro707 commented Jan 26, 2026 •
edited
Loading

This PR moves the classes GeekMagicSPIBus and Arduino_ST7789 away from dynamically allocated towards statically allocated.

There should be no reason to dynamically allocate them nor check for their readiness in run-time: the setup function calls the lcd init function, and from there are initialized throughout the lifecycle of the LCD. The init function itself cannot fail as it just writes to the spi bus.

This PR also enables a 2 second watchdog timer, in case something in loop() craps out. Needs more testing for this.

Also as a side note, why the yield calls inside lcdRunVendorInit? At this stage nothing else should really be running on the MCU that would require preemptive tasking (mainly wifi).

@Times-Z
Copy link
Owner

Times-Z commented Jan 26, 2026

I’ll take a look at this as soon as I have some time (not easy this week)
Indeed, there are lots of things I’m learning, whether in C++ or in the ESP/Arduino ecosystem, and I’m convinced there are plenty of things that could be improved or reconsidered in what I’ve started to write 😁

@Times-Z
Copy link
Owner

Times-Z commented Jan 31, 2026 •
edited
Loading

Edit : nice display is good

Can you fix the static analysis errors and we can merge that ? https://github.com/Times-Z/GeekMagic-Open-Firmware/actions/runs/21385074066/job/61559824953?pr=21

@Electro707 Electro707 mentioned this pull request Jan 31, 2026
Closed
@Electro707
Turns out CS is just tied low in HW, so no need for any special bus h…
3bbe83a 
…andler. Removed tft library's init as we are doing our own init anyways
@Electro707
Made clang-tidy happy
860bd69
@Electro707
Copy link
Contributor Author

Electro707 commented Feb 1, 2026 •
edited
Loading

From the discussion in #23, turns out that CS is not being driven anyways. So there is not need for a custom SPI handler, the parent class Arduino_HWSPI directly works.
Tested on a cube-lite model. But I think the same applies to the TV model, as looking at the disassembly picture pin 15 is actually just tied to GND directly. So driving that pin is actually probably killing that IO if the esp8266 doesn't protect itself well.
image

@Times-Z
Copy link
Owner

Times-Z commented Feb 1, 2026

I planned to try this this in the beginning of the week

Nice work !! I didn't find this tricks on my side (since I'm new in the world)

Times-Z
Times-Z approved these changes Feb 2, 2026
View reviewed changes
Copy link
Owner

@Times-Z Times-Z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating doc as well !

@Times-Z Times-Z merged commit 5a06e1a into Times-Z:develop Feb 2, 2026
4 checks passed
@github-actions
Copy link

github-actions bot commented Feb 2, 2026

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Times-Z Times-Z Times-Z approved these changes

Assignees

No one assigned

Labels

released

Projects

@Times-Z's GeekMagic-Open-Firmware
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Electro707 @Times-Z